Skip to content

Comments

Birmingham | ITP-Jan | Roman Sanaye | Sprint 2 | Module-Structuring-and-Testing-Data coursework#978

Open
RomanSanaye wants to merge 1 commit intoCodeYourFuture:mainfrom
RomanSanaye:acoursework/sprint-2-clean
Open

Birmingham | ITP-Jan | Roman Sanaye | Sprint 2 | Module-Structuring-and-Testing-Data coursework#978
RomanSanaye wants to merge 1 commit intoCodeYourFuture:mainfrom
RomanSanaye:acoursework/sprint-2-clean

Conversation

@RomanSanaye
Copy link

Learners, PR Template

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Changelist

  • Reviewed and worked through all JS files in the following folders:

    1. 1-key-errors
    2. 2-mandatory-debug
    3. 3-mandatory-implement
    4. 4-mandatory-interpret
    5. 5-stretch-extend
  • Made predictions, explained code, and debugged issues where needed

  • In 5-stretch-extend, handled edge cases in formatAs12HourClock (midnight, noon, leading zeros) and added comprehensive tests

  • All files have been reviewed and tested; outputs are verified

@RomanSanaye RomanSanaye added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 17, 2026
@LonMcGregor LonMcGregor added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Feb 23, 2026
Copy link

@LonMcGregor LonMcGregor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your solutions are all good, but several times you have left console.log calls within the function. Is this in line with best practices?

// =============> write your new code here
function convertToPercentage(decimalNumber) {
const percentage = `${decimalNumber * 100}%`;
console.log(`Your Decimal number will be ${percentage}.`);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you imagine any issues that might happen by including this console.log here inside the function?

Would you want it to always print to the console every time you run this function?

} No newline at end of file
let heightSquare = height * height;
let bmi = weight / heightSquare;
console.log(`Your BMI is ${bmi.toFixed(1)}`)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my earlier comment. Leaving console.log statements inside functions might not be the best approach

// This is the latest solution to the problem from the prep.
// Make sure to do the prep before you do the coursework
// Your task is to write tests for as many different groups of input data or edge cases as you can, and fix any bugs you find.
// --- > Debugging this code was not easy for me, I could solve this by the help of AI Explaining each step and conditions and finally we made it work well with different inputs, I mean string with number character.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using AI to help at this stage can help you get a feel for the javascript, you might want to try doing this again by yourself - with no AI help - to make sure you learned the steps needed. For now though, the solution you have found works well.

@LonMcGregor LonMcGregor added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Feb 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Reviewed Volunteer to add when completing a review with trainee action still to take.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants